test: expand RhythmBlocks test suite coverage (#5607) by moksha-hub · Pull Request #5772 · sugarlabs/musicblocks

moksha-hub · 2026-02-17T14:32:42Z

Summary

This PR significantly expands the test suite for RhythmBlocks, addressing the test coverage expansion request in issue #5607.

Changes Made

Added 15+ new test cases in js/blocks/__tests__/RhythmBlocks.test.js covering:

Edge Cases

MyNoteValueBlock handles undefined connections gracefully
SkipNotesBlock handles zero and negative skip values
MultiplyBeatFactorBlock handles fractional factors (0.5)
RhythmicDotBlock and RhythmicDot2Block handle single and multiple dots

Block Validation

NewNoteBlock registration and argument count validation
NoteBlock macros (note1-note8) definitions verification
OctaveSpaceBlock and DefineFrequencyBlock existence checks
Rhythm blocks palette assignment verification

Flow Block Testing

Flow blocks return correct tuple format [result, isFlowComplete]
Validated across multiple block types (swing, newswing2, skipnotes, etc.)

Error Handling

Missing statusFields handling in status matrix mode
Missing singer object on turtle handling
Null blockList entries handling

Testing

All new tests pass successfully:

npm test -- js/blocks/__tests__/RhythmBlocks.test.js

Coverage Impact

Increased edge case coverage for RhythmBlocks
Added defensive programming tests for error conditions
Validated block registration and configuration

Related Issues

Fixes #5607

Checklist

Added comprehensive test cases
All tests pass locally
Follows existing test patterns
Covers edge cases and error conditions

github-actions · 2026-02-17T14:33:54Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

RhythmBlocks.test.js

github-actions · 2026-02-17T14:47:07Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

RhythmBlocks.test.js

github-actions · 2026-02-17T15:04:42Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

RhythmBlocks.test.js

vanshika2720 · 2026-02-17T18:29:38Z

@moksha-hub Jest cases are failing.

omsuneri · 2026-02-18T08:50:50Z

@moksha-hub please resolve the failing test cases !!

github-actions · 2026-02-18T11:08:15Z

✅ All Jest tests passed! This PR is ready to merge.

github-actions · 2026-02-18T11:27:55Z

✅ All Jest tests passed! This PR is ready to merge.

github-actions · 2026-02-18T11:42:08Z

✅ All Jest tests passed! This PR is ready to merge.

moksha-hub · 2026-02-18T11:42:53Z

@vanshika2720 @omsuneri can you check now.
Thank you.

omsuneri · 2026-02-19T05:24:51Z

@moksha-hub i m seeing some changes unrelated to this pr like the package.json is update please rebase your branch first before commiting !!

github-actions · 2026-02-20T08:03:29Z

✅ All Jest tests passed! This PR is ready to merge.

github-actions · 2026-02-20T08:13:24Z

✅ All Jest tests passed! This PR is ready to merge.

moksha-hub · 2026-02-20T08:16:59Z

@omsuneri Rebased onto the latest master now contains only the test file changes in a single clean commit. Package file changes have been removed.
@vanshika2720 Please have a review.
Thank you

vanshika2720

@moksha-hub a few tests use conditional checks like if (block) or if (result) which may allow silent passes.
It would be better to assert existence explicitly to prevent regressions.

github-actions · 2026-02-20T17:27:30Z

✅ All Jest tests passed! This PR is ready to merge.

Added 15+ new test cases covering edge cases, block validation, flow block behaviour, and error handling. All conditional guards replaced with explicit expect() assertions per review feedback. Fixes sugarlabs#5607

github-actions · 2026-02-20T17:32:28Z

✅ All Jest tests passed! This PR is ready to merge.

moksha-hub · 2026-02-20T17:33:35Z

@vanshika2720 Replaced the conditional if (block) / if (result) guards with explicit expect().toBeDefined() assertions. Also corrected the flow-tuple test to only cover blocks that actually return tuples. All tests passing.

moksha-hub · 2026-02-21T05:38:27Z

@vanshika2720 @omsuneri may i know place to discuss regarding any doubts on Gsoc 2026 as i am aiming for it.
opened 2prs, 1(merged).
Thanks for your time.

vanshika2720

@moksha-hub Thanks for the updates.

I still see conditional checks like if (block) and if (block && block.flow) in the tests. Please replace them with explicit assertions (expect(...).toBeDefined()) to avoid silent passes.

Also, for GSoC 2026 queries, please use the Sugar Labs GSoC Discussions:
https://github.com/sugarlabs/GSoC/discussions

…ocks

github-actions · 2026-02-22T15:45:14Z

✅ All Jest tests passed! This PR is ready to merge.

github-actions · 2026-02-22T15:55:17Z

✅ All Jest tests passed! This PR is ready to merge.

moksha-hub · 2026-02-22T15:59:33Z

Thanks for your time.

moksha-hub · 2026-02-24T07:08:21Z

@walterbender sir please have a review. And already it was reviewed by @vanshika2720

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 71b8967 to 5fe791d Compare February 17, 2026 14:45

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 5fe791d to baafe3f Compare February 17, 2026 15:03

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 3d36fda to 556e2b5 Compare February 20, 2026 08:02

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 556e2b5 to 5f9e138 Compare February 20, 2026 08:12

vanshika2720 suggested changes Feb 20, 2026

View reviewed changes

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 5f9e138 to 3d36fda Compare February 20, 2026 17:26

test: expand RhythmBlocks test suite coverage (sugarlabs#5607)

5391abf

Added 15+ new test cases covering edge cases, block validation, flow block behaviour, and error handling. All conditional guards replaced with explicit expect() assertions per review feedback. Fixes sugarlabs#5607

moksha-hub force-pushed the expand-rhythm-blocks-tests-5607 branch from 3d36fda to 5391abf Compare February 20, 2026 17:31

moksha-hub requested a review from vanshika2720 February 20, 2026 17:33

vanshika2720 suggested changes Feb 22, 2026

View reviewed changes

test: replace conditional guards with explicit assertions in RhythmBl…

82f23dd

…ocks

style: run Prettier on RhythmBlocks.test.js

21e6881

moksha-hub requested a review from vanshika2720 February 22, 2026 15:59

Uh oh!

Conversation

moksha-hub commented Feb 17, 2026

Summary

Changes Made

Edge Cases

Block Validation

Flow Block Testing

Error Handling

Testing

Coverage Impact

Related Issues

Checklist

Uh oh!

github-actions bot commented Feb 17, 2026

Uh oh!

github-actions bot commented Feb 17, 2026

Uh oh!

github-actions bot commented Feb 17, 2026

Uh oh!

vanshika2720 commented Feb 17, 2026

Uh oh!

omsuneri commented Feb 18, 2026

Uh oh!

github-actions bot commented Feb 18, 2026

Uh oh!

github-actions bot commented Feb 18, 2026

Uh oh!

github-actions bot commented Feb 18, 2026

Uh oh!

moksha-hub commented Feb 18, 2026

Uh oh!

omsuneri commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 20, 2026

Uh oh!

github-actions bot commented Feb 20, 2026

Uh oh!

moksha-hub commented Feb 20, 2026

Uh oh!

vanshika2720 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 20, 2026

Uh oh!

github-actions bot commented Feb 20, 2026

Uh oh!

moksha-hub commented Feb 20, 2026

Uh oh!

moksha-hub commented Feb 21, 2026

Uh oh!

vanshika2720 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 22, 2026

Uh oh!

github-actions bot commented Feb 22, 2026

Uh oh!

moksha-hub commented Feb 22, 2026

Uh oh!

moksha-hub commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants